Change pcap file upload logging to be more descriptive. #285
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changing error log for file upload when there's an invalid file type uploaded. Previously just printed a single trashcan emoji (🗑️), changed to be more descriptive.
Fixed uploade -> uploaded typo
|
Unsure of labels or how to add them at the moment seeing it's greyed out for me. |
|
Thanks for the suggestion! There is a reason for the emojis: sometimes when I'm developing/debugging artifact processing I'm looking at hundreds or thousands of files, and they're zipping by in the logs quickly. Those icons (which I use across many of the internal malcolm scripts) make it easier to visually identify the operation at a glance. But I do agree that in this case the message needs to be more helpful to the end user as well. I think I will probably retain the icon but also add a more descriptive error message as you suggested. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🗣 Description
Changing error log for file upload when there's an invalid file type uploaded. Previously just printed a single trashcan emoji (🗑️), changed to be more descriptive.
💭 Motivation and context
When debugging why I couldn't upload a PCAP file to Malcolm I kept getting the below error.
This is minimally descriptive only providing a trashcan emoji along with the file name and mime type. The message can be assumed that your submitted file was immediately deleted. While the attached filename and mime type seems much more descriptive of what was uploaded, but not to why the error occurred.
This pull request changed this single line error to two much more descriptive errors:
While I like the flair emojis give scripts, putting them into error logs is a misstep and either further obfuscates the error at hand, or makes it much more difficult to parse.
🧪 Testing
Splits one print statement into two, removes an emoji, don't really think this applies.
✅ Pre-approval checklist
to reflect the changes in this PR.
✅ Pre-merge checklist
✅ Post-merge checklist